Codecov Report

Merging #18662 (e65142c) into master (bc6aad0) will increase coverage by 0.00%.
The diff coverage is 55.00%.

@@           Coverage Diff           @@
##           master   #18662   +/-   ##
=======================================
  Coverage   66.51%   66.51%           
=======================================
  Files        1644     1646    +2     
  Lines       63491    63530   +39     
  Branches     6458     6466    +8     
=======================================
+ Hits        42231    42258   +27     
- Misses      19590    19596    +6     
- Partials     1670     1676    +6     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc6aad0...e65142c. Read the comment docs.

Pinging @suddjian for a review since he's the SME for the fuzzy search aspect.

@stephenLYZ @yousoph @kasiazjc Let's test an ephemeral environment here (when CI is finished) and then we can probably remove the labels from the plugins here (just leaving the functionality for this PR). We want to leverage this mainly for Preset purposes, so we can configure it there.

@graceguo-supercat @amitmiran137 (or anyone looking at this):
Just FYI, we're planning to label our best-supported viz components using this feature. We're assuming others won't want these labels added on the repo... but if you think it's of general value, that makes our life easier! The list we intend to use is (correct me if I'm wrong, anyone):

• Big Num w Trendline
• Big Number
• Table
• Pivot Table v2
• Time Series Line
• Time Series Area
• Time Series Bar v2
• Time Series Scatter
• Pie
• Bar
• World Map

/testenv up

@geido Ephemeral environment spinning up at http://34.217.75.91:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@stephenLYZ This is looking great! I know this sounds weird, but let's take the metadata out of the charts for now, so we can merge the feature code of this PR.

Then we at Preset (or any other org) can add the labels we want, and not press anything upon the community quite yet. We can bring this up for wider discussion as a feature, to gain broader consensus around which charts are worth highlighting for the OSS releases.

/testenv up

@rusackas Ephemeral environment spinning up at http://34.222.147.29:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

More screenshots and implementation examples to add to the thread for posterity:

Multiple species of labels:

Weighting seems to work:

And to implement:

• Import ChartLabel from @superset-ui/core
• within a chart's new ChartMetadata object add:

label: {
  name: ChartLabel.VERIFIED, // or others from the ChartLabel enum
  description: t(
    'This chart was tested and verified, so the overall experience should be stable.',
  ),
},

• add to ChartLabelWeight any new label that should affect the sort order of search results

Future considerations:

• We may want to apply these weights not just to the search results, but to the general category listings or "All charts" list. I'm actually a little confused about the order that these are currently in anyway, but it seems to be neither by alphabetical order or by popularity. @kasiazjc may want to provide input for a future iteration.
• We could provide a default description (like you did with the weights) but still allow users to override this by populating the description.
  • We should to a little community outreach and see if there's alignment on which of these should have labels applied for all open source users

@rusackas Thanks for these suggestions! I agree to put this chat label into the weighted calculation of Fuse, which I tried but it did not succeed. The reason is that fuse calculates the score based on the key (https://github.com/krisk/Fuse/blob/master/src/core/computeScore.js), like 'name' or 'description', but in our scenario, it is based on the value, like 'verified' or 'feature'.

There is one approach I have in mind is to use: make this label as key, and value is the same as name

metadata: {
  name: t('Pie Chart'),
  verified: t('Pie Chart'),
  // or
  featured: t('Pie Chart')
}

So we can define the weights of keys:

new Fuse(chartMetadata, {
  ignoreLocation: true,
  threshold: 0.3,
  keys: [
    {
      name: 'value.verified',
      weight: 1.1
    },
    {
      name: 'value.featured',
      weight: 0.9
    },
    'value.name',
    'value.tags',
    'value.description'
  ],
}),
 

But this is not flexible. So there is a balance here.

Yeah, I can't think of a clear way to have these numeric weights influence the text-based weighted keys of fuse, either. Let's consider that for a follow-up, I think this PR moves the ball in the right direction far enough for now.

Ephemeral environment shutdown and build artifacts deleted.

🏷️ preset:2022.11